-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docker-container: restart-policy opt #1271
Conversation
LGTM. Personally, not sure about defaulting to |
Good point for |
PTAL @tonistiigi |
Needs rebase |
fd85125
to
81ad6b3
Compare
What is the use case for this? In what case would the user need to change this? Running a builder takes resources and memory so if it is not being used it is better if it is not running. |
@tonistiigi Main use case is for the Build UI in Docker Desktop so I can see my build history without needing to start my builders. |
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
81ad6b3
to
9822409
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think opt-in this is fine but I would be against making this default behavior.
@@ -44,17 +44,18 @@ type Driver struct { | |||
|
|||
// if you add fields, remember to update docs: | |||
// https://github.com/docker/docs/blob/main/content/build/drivers/docker-container.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this PR might've missed updating these docs 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's here :-) docker/docs#19355
Currently we can't set the restart policy for buildkit containers created with
docker-container
driver. In my use case I have restarted some machines but as the default restart policy isno
, the containers were not running. Even if we ensure nodes are booted before building I think it's still useful to have this driver opt.Current behavior (
unless-stopped
) is kept.Needs docs update in follow-up (cc @dvdksn): https://docs.docker.com/build/drivers/docker-container/#synopsis
Signed-off-by: CrazyMax crazy-max@users.noreply.github.com